refactor: map additional device export fields - #1233
Conversation
6ff249c to
97e79d6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## device-discovery-exportAPI #1233 +/- ##
==============================================================
+ Coverage 58.20% 58.39% +0.18%
==============================================================
Files 150 150
Lines 12329 12404 +75
==============================================================
+ Hits 7176 7243 +67
- Misses 5152 5160 +8
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The export shaping currently contains a non-compiling struct type conversion and a couple of export-presence/mapping edge cases that can drop or emit empty subsystems incorrectly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Extends the /api/v1/devices/export shaping to include additional OS/ME/platform network-related fields being synced from rpc-go, while preserving legacy fallback behavior for flat IP fields.
Changes:
- Adds nested DTO fields for OS DNS suffix, ME/OS network details, and platform adapter names on
DeviceInfo. - Updates export shaping to emit ME/OS network structures (with legacy fallback to flat IP fields) and platform adapters.
- Extends export endpoint test coverage to validate the newly-exported fields.
File summaries
| File | Description |
|---|---|
| internal/usecase/devices/usecase.go | Adds merge/setter keys for new device-info fields so partial updates can persist them. |
| internal/entity/dto/v1/device.go | Introduces new nested DeviceInfo DTO fields/types for OS DNS suffix, ME/OS network, and platform adapters. |
| internal/controller/httpapi/v1/export.go | Maps the new nested fields into the export response while retaining legacy fallbacks. |
| internal/controller/httpapi/v1/export_test.go | Adds assertions for DNS suffix, interface metadata, and platform adapters in export output. |
Review details
Suppressed comments (1)
internal/controller/httpapi/v1/export.go:224
buildExportOSdecides whether to emit the OS subsystem usinghasOS, but it doesn't include the newly-addedDNSSuffixOSorOSNetworkfields. If those are the only OS details present, the OS subsystem will incorrectly render as null even though data exists.
hasOS := info.OSName != "" || info.OSVersion != "" || info.OSDistro != "" ||
info.OSIPAddress != "" || info.LMSInstalled != nil || info.LMSVersion != "" ||
info.MEInterfaceVersion != "" || info.MonitorConnected != nil || info.IEEE8021XEnabled != nil
if !hasOS {
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
97e79d6 to
651194c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The export mapping currently skips legacy IP fallbacks (and can suppress OS export entirely) when nested network/dns fields are present-but-empty or are the only OS data, which contradicts the intended backward-compatible behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
internal/controller/httpapi/v1/export.go:242
- Same fallback issue as ME: if
info.OSNetworkis non-nil but empty,buildExportOSNetworkreturns nil and the legacyOSIPAddressfallback is skipped, resulting in a missing OS network in the export. Only prefer the nested mapping when it yields a non-nil export network.
if info.OSNetwork != nil {
osInfo.Network = buildExportOSNetwork(info.OSNetwork)
} else if info.OSIPAddress != "" {
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
651194c to
2ba5c3a
Compare
2ba5c3a to
94caa4f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
buildExportOS currently treats an empty-but-non-nil osNetwork object as OS presence, which can cause an “empty” OS subsystem to be exported instead of null when no OS data was actually reported.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
0062372 to
837de43
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new 1:1 struct-literal mappings in export.go are likely to fail CI due to staticcheck S1016 and should be replaced with direct struct conversions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
internal/controller/httpapi/v1/export.go:298
- This 1:1 field-copy struct literal is a candidate for staticcheck S1016. Since OSInterfaceInfo and ExportOSInterface have identical fields, prefer a direct conversion to avoid lint failures and reduce boilerplate.
return &dto.ExportOSInterface{
Name: adapter.Name,
IPAddress: adapter.IPAddress,
DHCPEnabled: adapter.DHCPEnabled,
LinkStatus: adapter.LinkStatus,
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
837de43 to
c8c7d4f
Compare
c8c7d4f to
56d646e
Compare
120a2f4 to
87748a5
Compare
87748a5 to
5b4dcf5
Compare
* persist ME and OS network details from rpc-go sync * map OS DNS suffix and platform adapters into export output * preserve fallback mapping for legacy flat IP fields * map the existing AMT BIOS state to the export MEBX field Addresses device-management-toolkit/rpc-go#1513
d476901 to
952a937
Compare
Addresses device-management-toolkit/rpc-go#1513
Dependencies
/api/v1/devices/exportendpoint and export DTOs.Implementation Details:
deviceInfo.dnsSuffixOSDeviceInfoand exported asdeviceInfo.os.dnsSuffix.deviceInfo.meNetwork.wireddeviceInfo.me.network.wiredwhen it contains at least one populated ME interface field.deviceInfo.meNetwork.wirelessdeviceInfo.me.network.wirelesswhen it contains at least one populated ME interface field.deviceInfo.meNetwork.*.ipAddressdeviceInfo.me.network.*.ipAddress.deviceInfo.meNetwork.*.dhcpEnableddeviceInfo.me.network.*.dhcpEnabled.deviceInfo.meNetwork.*.dhcpModedeviceInfo.me.network.*.dhcpMode.deviceInfo.meNetwork.*.linkStatusdeviceInfo.me.network.*.linkStatus.deviceInfo.meNetwork.*.macAddressdeviceInfo.me.network.*.macAddress.deviceInfo.ipAddress/deviceInfo.dhcpEnabledmeNetworkis absent or contains no usable interface data.deviceInfo.osNetwork.wired[]deviceInfo.os.network.wired[]; empty wired interface objects are ignored.deviceInfo.osNetwork.wirelessdeviceInfo.os.network.wirelesswhen it contains at least one populated OS interface field.deviceInfo.osNetwork.*.namedeviceInfo.os.network.*.name.deviceInfo.osNetwork.*.ipAddressdeviceInfo.os.network.*.ipAddress.deviceInfo.osNetwork.*.dhcpEnableddeviceInfo.os.network.*.dhcpEnabled; may benullwhen rpc-go cannot determine DHCP state.deviceInfo.osNetwork.*.linkStatusdeviceInfo.os.network.*.linkStatus.deviceInfo.osNetwork.*.macAddressdeviceInfo.os.network.*.macAddress.deviceInfo.osIpAddressosNetworkis absent or contains no usable interface data.deviceInfo.platformAdapters.wired[]deviceInfo.platform.adapters.wired[], containing all detected wired adapter display names.deviceInfo.platformAdapters.wireless[]deviceInfo.platform.adapters.wireless[], containing all detected wireless adapter display names.platformAdapters.wired/platformAdapters.wirelessdeviceInfo.ethernetAdapterCountdeviceInfo.platform.ethernetAdapterCount; it represents the total physical wired and wireless adapter count.deviceInfo.amtEnabledInBIOSdeviceInfo.me.mebxEnabledInBIOS; no duplicate storedmebxEnabledInBIOSfield is introduced.meNetwork/osNetworkobjectsdeviceInfo.meremainsnull.